Skip to content

Add timeout when watching pod events in k8s executor#39551

Merged
dstandish merged 4 commits into
apache:mainfrom
astronomer:add-timeout-when-reading-pod-events
May 15, 2024
Merged

Add timeout when watching pod events in k8s executor#39551
dstandish merged 4 commits into
apache:mainfrom
astronomer:add-timeout-when-reading-pod-events

Conversation

@dstandish
Copy link
Copy Markdown
Contributor

If we don't set a timeout, it may hang indefinitely if there's a network issue.

@dstandish dstandish requested a review from RNHTTR May 10, 2024 19:41
@boring-cyborg boring-cyborg Bot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels May 10, 2024
Copy link
Copy Markdown
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py Outdated
Comment thread airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py Outdated
Comment thread airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py Outdated
Comment thread airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py Outdated
Copy link
Copy Markdown
Contributor

@dirrao dirrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

dstandish and others added 3 commits May 13, 2024 22:17
If we don't set a timeout, it may hang indefinitely if there's a network issue.
…r_utils.py

Co-authored-by: Ryan Hatter <25823361+RNHTTR@users.noreply.github.com>
@dstandish dstandish force-pushed the add-timeout-when-reading-pod-events branch from 654739b to 54a3fe0 Compare May 14, 2024 06:13
@collinmcnulty
Copy link
Copy Markdown
Contributor

Should this be marked as closes #36998 ?

@dstandish
Copy link
Copy Markdown
Contributor Author

Should this be marked as closes #36998 ?

it's not obvious to me how this would be connected to that issue

Comment on lines +156 to +157
kwargs["_request_timeout"] = client_timeout
kwargs["timeout_seconds"] = server_conn_timeout
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These settings were configurable using the AIRFLOW__KUBERNETES_EXECUTOR__KUBE_CLIENT_REQUEST_ARGS env variable, but now they are overridden if I'm not mistaken. Is this expected?

@dstandish dstandish added this to the Airflow 2.10.0 milestone Jul 18, 2024
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
If we don't set a timeout, it may hang indefinitely if there's a network issue.

---------

Co-authored-by: Ryan Hatter <25823361+RNHTTR@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants